feat: honor thread context classloader when loading custom normalizers and generators - #24617
feat: honor thread context classloader when loading custom normalizers and generators#24617Picazsoo wants to merge 16 commits into
Conversation
…t classloader support
There was a problem hiding this comment.
All reported issues were addressed across 8 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
- Keep core error message tool-agnostic (no Gradle-specific terms); Gradle-specific guidance remains only in the plugin README/docs. - Add missing test coverage for generatorClasspath + classloader isolation. - Clean up temp directories created by classloader fallback tests to avoid leaking compiled fixture files across test runs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…lizer KDoc Cross-reference generatorClasspath/openApiGeneratorExtra directly on the openapiNormalizer property (both extension and task) so the requirement is discoverable from IDE tooltips/KDoc, not just the README. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
All reported issues were addressed across 8 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
- Split ClassNotFoundException (classpath guidance) from other ReflectiveOperationException cases (constructor/instantiation failures) in OpenAPINormalizer.createNormalizer so the error message matches the actual failure. - Replace redundant expectedExceptions + manual catch/rethrow in OpenAPINormalizerTest with expectThrows. - Clean up normalizer fixture temp directories after each test in GeneratorClasspathIsolationTest (AfterMethod deleteRecursively). - Assert on the wrapped classpath-guidance log message in addition to the raw ClassNotFoundException text, reducing coupling to log format. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
All reported issues were addressed across 8 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
- Skip (not NPE) the TCCL-fixture test when running on a JRE without a system Java compiler (ToolProvider.getSystemJavaCompiler() returns null). - Assert that positive GeneratorClasspathIsolationTest cases show no NORMALIZER_CLASS load failure/ClassNotFoundException in the build log, in addition to TaskOutcome.SUCCESS, since DefaultGenerator logs but does not fail the build on a normalizer load failure. Without this, a regression dropping the forwarded classpath would pass silently. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The fixture normalizer now overrides normalize() to write a marker file (path passed via a MARKER_FILE inputRule) before delegating to super, giving direct proof the custom NORMALIZER_CLASS ran under the worker - across a forked JVM in 'process' isolation - rather than relying only on the build succeeding and no failure text appearing in the log. The negative-control test asserts the marker is absent when the class fails to load, corroborating that normalize() never executed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
All reported issues were addressed across 8 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
…fixture Mirrors the guard already present in OpenAPINormalizerTest's compileNormalizerFixture: skip with a clear SkipException instead of an opaque NullPointerException when running on a JRE without a system Java compiler. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ove error messages
There was a problem hiding this comment.
All reported issues were addressed across 5 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
There was a problem hiding this comment.
All reported issues were addressed across 3 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
…or class loading issues
There was a problem hiding this comment.
All reported issues were addressed across 2 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
…loading exceptions
There was a problem hiding this comment.
All reported issues were addressed across 1 file (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
…n failures during class loading
There was a problem hiding this comment.
All reported issues were addressed across 2 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
There was a problem hiding this comment.
3 issues found across 10 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="modules/openapi-generator-gradle-plugin/src/test/kotlin/GeneratorClasspathIsolationTest.kt">
<violation number="1" location="modules/openapi-generator-gradle-plugin/src/test/kotlin/GeneratorClasspathIsolationTest.kt:197">
P3: The build.gradle scripts and fixture-jar helpers are duplicated across 10 test methods, so a change to the wiring (e.g. a new isolation mode or property) or to the marker path must be edited in ~8 nearly identical Groovy snippets. Consider extracting a buildScript(workerIsolation, classpathSource, generatorName, extraProps) helper and a writeSpec/marker helper to keep the matrix readable and reduce drift risk when the configuration changes.</violation>
</file>
<file name="modules/openapi-generator/src/main/java/org/openapitools/codegen/CodegenConfigLoader.java">
<violation number="1" location="modules/openapi-generator/src/main/java/org/openapitools/codegen/CodegenConfigLoader.java:84">
P2: Selecting a valid generator can fail because an unrelated broken SPI provider on the TCCL throws during `provider.get()`, before direct-name resolution or the new `GeneratorNotFoundException` handling runs. Handling provider failures per entry (or deferring/isolating them until the requested provider is known) would preserve the requested generator path and its actionable error messages.</violation>
</file>
<file name="modules/openapi-generator-gradle-plugin/README.adoc">
<violation number="1" location="modules/openapi-generator-gradle-plugin/README.adoc:508">
P3: The new README sentence contains the grammatically incorrect phrase “forwarded from into `generatorClasspath`,” which makes the classpath forwarding relationship harder to understand. Rewording it to “automatically forwarded into” keeps the documentation clear.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| ServiceLoader<CodegenConfig> loader = ServiceLoader.load(CodegenConfig.class, classLoader); | ||
| loader.stream().forEach(provider -> { | ||
| if (configClasses.add(provider.type().getName())) { | ||
| output.add(provider.get()); |
There was a problem hiding this comment.
P2: Selecting a valid generator can fail because an unrelated broken SPI provider on the TCCL throws during provider.get(), before direct-name resolution or the new GeneratorNotFoundException handling runs. Handling provider failures per entry (or deferring/isolating them until the requested provider is known) would preserve the requested generator path and its actionable error messages.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At modules/openapi-generator/src/main/java/org/openapitools/codegen/CodegenConfigLoader.java, line 84:
<comment>Selecting a valid generator can fail because an unrelated broken SPI provider on the TCCL throws during `provider.get()`, before direct-name resolution or the new `GeneratorNotFoundException` handling runs. Handling provider failures per entry (or deferring/isolating them until the requested provider is known) would preserve the requested generator path and its actionable error messages.</comment>
<file context>
@@ -43,18 +50,129 @@ public static CodegenConfig forName(String name) {
+ ServiceLoader<CodegenConfig> loader = ServiceLoader.load(CodegenConfig.class, classLoader);
+ loader.stream().forEach(provider -> {
+ if (configClasses.add(provider.type().getName())) {
+ output.add(provider.get());
+ }
+ });
</file context>
| // Note: DefaultGenerator logs (but does not fail the build on) NORMALIZER_CLASS load | ||
| // failures - this is pre-existing behavior unrelated to this fix. Assert on the log | ||
| // output instead of the task outcome. | ||
| val result = runOpenApiGenerateExpectingSuccess( |
There was a problem hiding this comment.
P3: The build.gradle scripts and fixture-jar helpers are duplicated across 10 test methods, so a change to the wiring (e.g. a new isolation mode or property) or to the marker path must be edited in ~8 nearly identical Groovy snippets. Consider extracting a buildScript(workerIsolation, classpathSource, generatorName, extraProps) helper and a writeSpec/marker helper to keep the matrix readable and reduce drift risk when the configuration changes.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At modules/openapi-generator-gradle-plugin/src/test/kotlin/GeneratorClasspathIsolationTest.kt, line 197:
<comment>The build.gradle scripts and fixture-jar helpers are duplicated across 10 test methods, so a change to the wiring (e.g. a new isolation mode or property) or to the marker path must be edited in ~8 nearly identical Groovy snippets. Consider extracting a buildScript(workerIsolation, classpathSource, generatorName, extraProps) helper and a writeSpec/marker helper to keep the matrix readable and reduce drift risk when the configuration changes.</comment>
<file context>
@@ -0,0 +1,487 @@
+ // Note: DefaultGenerator logs (but does not fail the build on) NORMALIZER_CLASS load
+ // failures - this is pre-existing behavior unrelated to this fix. Assert on the log
+ // output instead of the task outcome.
+ val result = runOpenApiGenerateExpectingSuccess(
+ """
+ plugins { id 'org.openapi.generator' }
</file context>
| [NOTE] | ||
| ==== | ||
| The plugin creates an `openApiGeneratorExtra` dependency configuration (resolvable, not published) that entries | ||
| are automatically forwarded from into `generatorClasspath` for both `workerIsolation` modes. Use it to declare a |
There was a problem hiding this comment.
P3: The new README sentence contains the grammatically incorrect phrase “forwarded from into generatorClasspath,” which makes the classpath forwarding relationship harder to understand. Rewording it to “automatically forwarded into” keeps the documentation clear.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At modules/openapi-generator-gradle-plugin/README.adoc, line 508:
<comment>The new README sentence contains the grammatically incorrect phrase “forwarded from into `generatorClasspath`,” which makes the classpath forwarding relationship harder to understand. Rewording it to “automatically forwarded into” keeps the documentation clear.</comment>
<file context>
@@ -480,8 +480,56 @@ warning].
+[NOTE]
+====
+The plugin creates an `openApiGeneratorExtra` dependency configuration (resolvable, not published) that entries
+are automatically forwarded from into `generatorClasspath` for both `workerIsolation` modes. Use it to declare a
+custom `NORMALIZER_CLASS`, custom generator selected by `generatorName`/FQCN, or any other class referenced by name
+in generator options as a normal Gradle dependency - a published artifact, a local jar, or another project in the
</file context>
| are automatically forwarded from into `generatorClasspath` for both `workerIsolation` modes. Use it to declare a | |
| are automatically forwarded into `generatorClasspath` for both `workerIsolation` modes. Use it to declare a |
A custom
NORMALIZER_CLASSnormalizer could fail to load, especially under the Gradle plugin'sprocessworker isolation:OpenAPINormalizer.createNormalizerused bareClass.forName(String), which resolves against the classloader that loadedopenapi-generatorcore, not any user-supplied classpath.classpath.from(...)in either isolation branch. Underprocessisolation this is fatal: the forked JVM has zero visibility into anything not explicitly forwarded. Underclassloaderisolation it was merely fragile: since the worker runs in the same JVM, a custom normalizer could work by accident if it happened to already be reachable through the existing classloader hierarchy (e.g. stuck on the buildscript classpath), but this was never a supported or documented mechanism.Changes
Core (
OpenAPINormalizer.java)createNormalizernow resolves the class via the current thread's context classloader (TCCL) first (which Gradle's Worker API sets to include user classpath), falling back to the original defining classloader for backward compatibility.ClassNotFoundExceptiongets classpath guidance, other reflective failures (bad constructor, instantiation errors) get a distinct, accurate message.Gradle plugin
generatorClasspath(file collection) and anopenApiGeneratorExtraresolvable configuration, both forwarded to the worker inprocessIsolationandclassLoaderIsolation. Users can now supply a custom normalizer jar via normal Gradle dependencies or file collections.openapiNormalizerKDoc, README, and customization docs.Tests
GeneratorClasspathIsolationTestcovering both classpath mechanisms under both isolation modes, plus a negative control. Positive tests assert a marker file written by the fixture normalizer'snormalize()to directly prove it ran, not just that the build succeeded.Compatibility
Fully backward compatible. No
NORMALIZER_CLASSset, or normalizer already visible on the default classpath: unchanged behavior. New properties default to empty/unset.PR checklist
Commit all changed files.
This is important, as CI jobs will verify all generator outputs of your HEAD commit as it would merge with master.
These must match the expectations made by your contribution.
You may regenerate an individual generator by passing the relevant config(s) as an argument to the script, for example
./bin/generate-samples.sh bin/configs/java*.IMPORTANT: Do NOT purge/delete any folders/files (e.g. tests) when regenerating the samples as manually written tests may be removed.
Summary by cubic
Prefer the thread context classloader when loading custom
NORMALIZER_CLASSand custom generators, and forward user classpaths in the Gradle plugin for both isolation modes. Adds precise, tool-agnostic errors, better generator discovery, updated docs, and tests that prove the classes actually run.New Features
OpenAPINormalizernow resolves via the thread context classloader with a safe fallback and clearer errors (classpath/not‑found vs bad constructor).CodegenConfigLoaderuses the context classloader;getAll()merges/de‑dupes across classloaders;forName()returns precise errors for classpath/not‑found, incompatible Java version, static initializer failures (including subsequentNoClassDefFoundError), linkage errors, and bad constructors/implementations.openApiGeneratorExtraand ageneratorClasspathproperty; both are forwarded to the worker classpath forworkerIsolationprocessandclassloader. README/KDoc document classpath requirements forNORMALIZER_CLASSand custom generators by name/FQCN. Tests cover both isolation modes, generator by FQCN, negative controls, and execution proven via marker files.Migration
NORMALIZER_CLASSor a custom generator (by name/FQCN) with the Gradle plugin, add it toopenApiGeneratorExtra(preferred) orgeneratorClasspath, e.g.dependencies { openApiGeneratorExtra("com.acme:my-normalizer:1.0.0") }oropenApiGenerate { generatorClasspath.from(files("libs/my-generator.jar")) }.Written for commit c76fd40. Summary will update on new commits.